Detect JVM installs at startup #231

mickaelistria · 2023-04-04T15:37:57Z

Fixes #230

What it does

This adds a (opt-out) startup job that looks up installed JRE is some standard directores. So people usually don't need to configure JRE by hand. It's useful both to Eclipse IDE and JDT-LS with VSCode.

=## How to test

Start a fresh workspace, go to Installed JREs preference page, enjoy that some JREs are detected without any effort.

Author checklist

I have thoroughly tested my changes
The change is following the coding conventions
I have signed the Eclipse Contributor Agreement (ECA)

org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java

mickaelistria · 2023-04-14T12:16:20Z

All tests are now passing. I had to adjust some of them because those were assuming that no other JRE is configured for proper test executions; which is now wrong on CI as the infra has numerous JVMs that are now detected, some of them without sources. Adding extra checks on the tests make them work more reliably on any infra.

SarikaSinha · 2023-04-14T12:21:35Z

@mickaelistria
I will look at the code in the next few days. Had some basic queries :

Does it handle only to populate the JREs in Installed JREs block or also uses them to launch if there was none defined in ini or in the folder under the eclipse directory?
It does not break anything defined here https://wiki.eclipse.org/Eclipse.ini ?

mickaelistria · 2023-04-14T12:33:49Z

Does it handle only to populate the JREs in Installed JREs block

Yes.

also uses them to launch if there was none defined in ini or in the folder under the eclipse directory?

It adds the VMs so JDT known them, then JDT may then use it or not in order to run particular programs or resolve execution environments. I did not touch the code that is responsible for choosing the VM to use by default when computing a project build path or when running it. So if it's hardcoded somewhere than the one in eclipse.ini should have priority, it should still happen.

It does not break anything defined here https://wiki.eclipse.org/Eclipse.ini ?

The patch doesn't affect the Eclipse launch in any way. So what is defined in eclipse.ini and how it is used in order to start the Eclipse product is not at all modified by this patch.

org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/core/ClasspathVariableTests.java

org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/sourcelookup/Bug565462Tests.java

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/jres/JREsPreferencePage.java

...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java

mickaelistria · 2023-04-18T12:30:30Z

@iloveeclipse Any remaining issues you think need to be considered before we merge?
@SarikaSinha IIRC, you're using a Mac, aren't you? If so, and if you feel like it, feel free to enrich this PR with whatever makes it work better on mac.

iloveeclipse · 2023-04-18T12:50:22Z

Any remaining issues you think need to be considered before we merge?

Indeed, failing test :-)

https://ci.eclipse.org/jdt/job/eclipse.jdt.debug-github/job/PR-231/20/testReport/org.eclipse.jdt.debug.tests.sourcelookup/Bug565462Tests/testFindDuplicatesBug565462/

mickaelistria · 2023-04-18T12:55:35Z

Indeed, failing test :-)

Thanks for the reminder, I had forgotten about it and will try to fix it shortly.

iloveeclipse · 2023-05-15T17:34:51Z

@iloveeclipse Are you fine with @jjohnstn taking over?

No objections. However, I would prefer to postpone this and plan for 4.29 M1.

mickaelistria · 2023-06-12T07:46:23Z

Hi,
Can this be considered for a review/merge soon, so we get time to gather feedback during 4.29 cycle and adapt whatever needs to be adapted in a comfortable timeframe?

akurtakov · 2023-06-12T07:48:29Z

@jjohnstn Would you please review this one?

jjohnstn · 2023-06-12T18:43:49Z

/rebase

...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java

Fixes eclipse-jdt#230

In light of all conversations being resolved and Mac support being relegated to another issue, the changes have been addressed.

jjohnstn · 2023-06-15T18:49:40Z

@mickaelistria Please add a N&N entry. Patch approved.

iloveeclipse · 2023-06-16T05:41:06Z

Note: we see eclipse-jdt/eclipse.jdt.core#1150 today, may be caused by this PR.

* Remove use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage This change replaces use of OPTION_JdtDebugCompileMode with OPTION_IgnoreUnnamedModuleForSplitPackage, since the latter does what we need better and doesn't cause the side effects that the first option causes. Goal is to later on remove OPTION_JdtDebugCompileMode in JDT core. Fixes: #260 Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> * Detect JVM installs at startup (#231) * Detect JVM installs at startup Fixes #230 * Delete classpath argument files upon shutdown. Fixes #240 (#242) Store classpath argument files in bundle state location, not in workspace - Avoids that workspace is contaminated with temporary files. - Delete classpath argument files upon shutdown. - Version bump org.eclipse.jdt.launching. See https://github.com/eclipse-platform/eclipse.platform.debug/issues/84 Fixes #240 * Avoid reusing of not thread-safe SimpleDateFormat SimpleDateFormat.format(new Date()); returns same string as immutable and thread-safe DateTimeFormatter.format(new Date().toInstant()) * Avoid potential Threadlocal.set(null) memory leak (#249) Threadlocal.set(null) keeps a reference to ThreadLocal.this see https://rules.sonarsource.com/java/RSPEC-5164 Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Change how JrtPackageFragmentRoots are printed in Source tab (#266) - currently all JrtPackageFragmentRoots are printed using the path which is the same for all modules extracted from a jrt-fs.jar - change this to print the module name followed by the full jrt-fs.jar path - fixes #633 * JavaSnippetEditor: fix missing synchronize (#259) * JavaSnippetEditor: fix missing synchronize * JavaSnippetEditor: Adding "this." for non static field access --------- Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * LaunchingPlugin: use try-with-resource (#252) to close streams in case of Exceptions. Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * Add missing "static" modifier for constants (#256) To reduce memory per instance Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> * [trivial] Fixed "Redundant specification of type arguments" warnings Reported by official SDK build since 4.29 for whatever reason See for example https://download.eclipse.org/eclipse/downloads/drops4/I20230621-1800/compilelogs/plugins/org.eclipse.jdt.debug.jdi.tests_1.1.0.v20230328-1614/@dot.html#OTHER_WARNINGS * Records with inner records breaks breakpoint toggling fixes #270 Boundary for Breakpoint should be checked * Touch bundles affected by the new ecj version See eclipse-platform/eclipse.platform.releng.aggregator#1184 * Fix html links in VMInstall(Type) Extension-Point documentation * Re-throw underlying exceptions in OpenFromClipboardTests #27 (#279) Replace call to Display::syncExec with call to Display::syncCall in the private method getJavaElementMatches. snycCall does not hide exceptions, it re-throws them to the calling thread (which helps debugging). Don't catch exceptions in the helper class "Accessor", re-throw them so the calling tests can show a more meaningful stack-trace when they fail. * Use typed for-each loop Avoiding type-casts makes code easier to read. * Avoid wrapping primitives for toString() see https://rules.sonarsource.com/java/RSPEC-2131 * ReferenceTypeImpl: compare Array content instead of instance comparison. https://stackoverflow.com/questions/8777257/equals-vs-arrays-equals-in-java * VMInstallTests: fix random ConcurrentModificationException #202 by using threadsafe datastructures. * Set console stream encoding for java >= 19 - eclipse.platform.debug#124 https://github.com/eclipse-platform/eclipse.platform.debug/issues/124 * FileHashing: avoid getCanonicalFile() getCanonicalFile() is slow on windows / JDK17 - can costs even more then calculating the SHA1 of the file content, which it is supposed to avoid. * Instead use normalized path. It does not matter that multiple files could be the same due to symbolic links, it will just recalculate their SHA1 for all of them - which is still faster. * Also read all file attributes at once. * Refactor XML parsing * Fix path decoding of URL to File containing space (fixes #64) A space in an URL is encoded as "%20" and has to be decoded to " ". Otherwise a debugged JAR rooted in a path containing space is not opening class file but java file editor. * Add MacOS support for detecting VM installs at start up (#276) - fixes #265 * require org.eclipse.core.runtime 3.29.0 --------- Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Simeon Andreev <simeon.danailov.andreev@gmail.com> Co-authored-by: Mickael Istria <mistria@redhat.com> Co-authored-by: Diethard Ohrt <130975899+diti0023@users.noreply.github.com> Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de> Co-authored-by: Jörg Kubitz <51790620+jukzi@users.noreply.github.com> Co-authored-by: Jeff Johnston <jjohnstn@redhat.com> Co-authored-by: Andrey Loskutov <loskutov@gmx.de> Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net> Co-authored-by: Federico Jeanne <2205684+fedejeanne@users.noreply.github.com>

trancexpress reviewed Apr 4, 2023

View reviewed changes

org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java Outdated Show resolved Hide resolved

trancexpress reviewed Apr 4, 2023

View reviewed changes

org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java Outdated Show resolved Hide resolved

trancexpress reviewed Apr 4, 2023

View reviewed changes

org.eclipse.jdt.launching/launching/org/eclipse/jdt/internal/launching/LaunchingPlugin.java Outdated Show resolved Hide resolved

iloveeclipse requested changes Apr 4, 2023

View reviewed changes

mickaelistria force-pushed the auto-detect-jvm-installs branch 4 times, most recently from caaa072 to b96a86a Compare April 11, 2023 08:46

mickaelistria force-pushed the auto-detect-jvm-installs branch 10 times, most recently from fd33f8f to 1de400f Compare April 14, 2023 11:40

mickaelistria requested a review from iloveeclipse April 14, 2023 12:14

iloveeclipse previously requested changes Apr 14, 2023

View reviewed changes

mickaelistria force-pushed the auto-detect-jvm-installs branch 3 times, most recently from d81cb13 to 71f0d24 Compare April 17, 2023 15:58

mickaelistria force-pushed the auto-detect-jvm-installs branch from 71f0d24 to bbcb52c Compare April 18, 2023 13:33

mickaelistria mentioned this pull request May 24, 2023

Automatically add configured toolchains eclipse-m2e/m2e-core#939

Closed

mickaelistria force-pushed the auto-detect-jvm-installs branch from cf060d2 to 2e0d0eb Compare June 9, 2023 21:27

github-actions bot force-pushed the auto-detect-jvm-installs branch from 2e0d0eb to 225a51c Compare June 12, 2023 18:44

jjohnstn requested changes Jun 13, 2023

View reviewed changes

...pse.jdt.launching/launching/org/eclipse/jdt/internal/launching/DetectVMInstallationsJob.java Show resolved Hide resolved

Detect JVM installs at startup

8a23f4f

Fixes eclipse-jdt#230

mickaelistria force-pushed the auto-detect-jvm-installs branch from 1b474c6 to 8a23f4f Compare June 14, 2023 06:17

mickaelistria mentioned this pull request Jun 15, 2023

Detect JVM installs at startup on Mac #265

Closed

jjohnstn added 3 commits June 15, 2023 10:47

Bump o.e.jdt.debug

01dafca

Bump o.e.jdt.debug.ui

8897f79

Bump o.e.jdt.debug.tests

01b83d1

jjohnstn approved these changes Jun 15, 2023

View reviewed changes

jjohnstn added this to the 4.29 M1 milestone Jun 15, 2023

jjohnstn added enhancement New feature or request noteworthy Noteworthy feature labels Jun 15, 2023

jjohnstn merged commit 582c760 into eclipse-jdt:master Jun 15, 2023

iloveeclipse mentioned this pull request Jun 16, 2023

Test fails with I20230615-1800 eclipse-jdt/eclipse.jdt.core#1150

Closed

iloveeclipse mentioned this pull request Jun 19, 2023

Fix NPE in JRTUtil eclipse-jdt/eclipse.jdt.core#1155

Merged

3 tasks

rgrunber mentioned this pull request Jul 11, 2023

Use DetectVMInstallationsJob.disabled=true property from JDT Debug redhat-developer/quarkus-ls#898

Merged

BeckerWdf mentioned this pull request Aug 3, 2023

An internal error occurred during: "Look up for installed JVMs". #289

Closed

iloveeclipse mentioned this pull request Oct 13, 2023

API tooling goes amok in Jenkins, need some extra logging eclipse-pde/eclipse.pde#782

Closed

iloveeclipse mentioned this pull request May 22, 2024

Search in 4.30 sometimes finds nothing eclipse-jdt/eclipse.jdt.core#1488

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect JVM installs at startup #231

Detect JVM installs at startup #231

mickaelistria commented Apr 4, 2023

mickaelistria commented Apr 14, 2023

SarikaSinha commented Apr 14, 2023

mickaelistria commented Apr 14, 2023

mickaelistria commented Apr 18, 2023

iloveeclipse commented Apr 18, 2023

mickaelistria commented Apr 18, 2023

iloveeclipse commented May 15, 2023

mickaelistria commented Jun 12, 2023

akurtakov commented Jun 12, 2023

jjohnstn commented Jun 12, 2023

jjohnstn commented Jun 15, 2023

iloveeclipse commented Jun 16, 2023

Detect JVM installs at startup #231

Detect JVM installs at startup #231

Conversation

mickaelistria commented Apr 4, 2023

What it does

Author checklist

mickaelistria commented Apr 14, 2023

SarikaSinha commented Apr 14, 2023

mickaelistria commented Apr 14, 2023

mickaelistria commented Apr 18, 2023

iloveeclipse commented Apr 18, 2023

mickaelistria commented Apr 18, 2023

iloveeclipse commented May 15, 2023

mickaelistria commented Jun 12, 2023

akurtakov commented Jun 12, 2023

jjohnstn commented Jun 12, 2023

jjohnstn commented Jun 15, 2023

iloveeclipse commented Jun 16, 2023